Skip to content

Stabilize let chains in the 2024 edition #132833

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 22, 2025

Conversation

est31
Copy link
Member

@est31 est31 commented Nov 10, 2024

Stabilization report

This proposes the stabilization of let_chains (tracking issue, RFC 2497) in the 2024 edition of Rust.

What is being stabilized

The ability to &&-chain let statements inside if and while is being stabilized, allowing intermixture with boolean expressions. The patterns inside the let sub-expressions can be irrefutable or refutable.

struct FnCall<'a> {
    fn_name: &'a str,
    args: Vec<i32>,
}

fn is_legal_ident(s: &str) -> bool {
    s.chars()
        .all(|c| ('a'..='z').contains(&c) || ('A'..='Z').contains(&c))
}

impl<'a> FnCall<'a> {
    fn parse(s: &'a str) -> Option<Self> {
        if let Some((fn_name, after_name)) = s.split_once("(")
            && !fn_name.is_empty()
            && is_legal_ident(fn_name)
            && let Some((args_str, "")) = after_name.rsplit_once(")")
        {
            let args = args_str
                .split(',')
                .map(|arg| arg.parse())
                .collect::<Result<Vec<_>, _>>();
            args.ok().map(|args| FnCall { fn_name, args })
        } else {
            None
        }
    }
    fn exec(&self) -> Option<i32> {
        let iter = self.args.iter().copied();
        match self.fn_name {
            "sum" => Some(iter.sum()),
            "max" => iter.max(),
            "min" => iter.min(),
            _ => None,
        }
    }
}

fn main() {
    println!("{:?}", FnCall::parse("sum(1,2,3)").unwrap().exec());
    println!("{:?}", FnCall::parse("max(4,5)").unwrap().exec());
}

The feature will only be stabilized for the 2024 edition and future editions. Users of past editions will get an error with a hint to update the edition.

closes #53667

Why 2024 edition?

Rust generally tries to ship new features to all editions. So even the oldest editions receive the newest features. However, sometimes a feature requires a breaking change so much that offering the feature without the breaking change makes no sense. This occurs rarely, but has happened in the 2018 edition already with async and await syntax. It required an edition boundary in order for async/await to become keywords, and the entire feature foots on those keywords.

In the instance of let chains, the issue is the drop order of if let chains. If we want if let chains to be compatible with if let, drop order makes it hard for us to generate correct MIR. It would be strange to have different behaviour for if let ... {} and if true && let ... {}. So it's better to stay consistent with if let.

In edition 2024, drop order changes have been introduced to make if let temporaries be lived more shortly. These changes also affected if let chains. These changes make sense even if you don't take the if let chains MIR generation problem into account. But if we want to use them as the solution to the MIR generation problem, we need to restrict let chains to edition 2024 and beyond: for let chains, it's not just a change towards more sensible behaviour, but one required for correct function.

Introduction considerations

As edition 2024 is very new, this stabilization PR only makes it possible to use let chains on 2024 without that feature gate, it doesn't mark that feature gate as stable/removed. I would propose to continue offering the let_chains feature (behind a feature gate) for a limited time (maybe 3 months after stabilization?) on older editions to allow nightly users to adopt edition 2024 at their own pace. After that, the feature gate shall be marked as stabilized, not removed, and replaced by an error on editions 2021 and below.

Implementation history

Adoption history

In the compiler

Outside of the compiler

Tests

Intentional restrictions

partially-macro-expanded.rs, macro-expanded.rs: it is possible to use macros to expand to both the pattern and the expression inside a let chain, but not to the entire let pat = expr operand.
parens.rs: if (let pat = expr) is not allowed in chains
ensure-that-let-else-does-not-interact-with-let-chains.rs: let...else doesn't support chaining.

Overlap with match guards

move-guard-if-let-chain.rs: test for the use moved value error working well in match guards. could maybe be extended with let chains that have more than one let
shadowing.rs: shadowing in if let guards works as expected
ast-validate-guards.rs: let chains in match guards require the match guards feature gate

Simple cases from the early days

PR #88642 has added some tests with very simple usages of let else, mostly as regression tests to early bugs.

then-else-blocks.rs
ast-lowering-does-not-wrap-let-chains.rs
issue-90722.rs
issue-92145.rs

Drop order/MIR scoping tests

issue-100276.rs: let expressions on RHS aren't terminating scopes
drop_order.rs: exhaustive temporary drop order test for various Rust constructs, including let chains
scope.rs: match guard scoping test
drop-scope.rs: another match guard scoping test, ensuring that temporaries in if-let guards live for the arm
drop_order_if_let_rescope.rs: if let rescoping on edition 2024, including chains
mir_let_chains_drop_order.rs: comprehensive drop order test for let chains, distinguishes editions 2021 and 2024.
issue-99938.rs, issue-99852.rs both bad MIR ICEs fixed by #102394

Linting

irrefutable-lets.rs: trailing and leading irrefutable let patterns get linted for, others don't. The lint is turned off for else if.
issue-121070-let-range.rs: regression test for false positive of the unused parens lint, precedence requires the ()s here

Parser: intentional restrictions

disallowed-positions.rs: let in expression context is rejected everywhere except at the top level
invalid-let-in-a-valid-let-context.rs: nested let is not allowed (let's are no legal expressions just because they are allowed in if and while).

Parser: recovery

issue-103381.rs: Graceful recovery of incorrect chaining of if and if let
semi-in-let-chain.rs: Ensure that stray ;s in let chains give nice errors (if_chain! users might be accustomed to ;s)
deli-ident-issue-1.rs, brace-in-let-chain.rs: Ensure that stray unclosed {s in let chains give nice errors and hints

Misc

conflicting_bindings.rs: the conflicting bindings check also works in let chains. Personally, I'd extend it to chains with multiple let's as well.
let-chains-attr.rs: attributes work on let chains

Tangential tests with #![feature(let_chains)]

if-let.rs: MC/DC coverage tests for let chains
logical_or_in_conditional.rs: not really about let chains, more about dropping/scoping behaviour of ||
stringify.rs: exhaustive test of the stringify macro
expanded-interpolation.rs, expanded-exhaustive.rs: Exhaustive test of -Zunpretty
diverges-not.rs: Never type, mostly tangential to let chains

Possible future work

  • There is proposals to allow if let Pat(bindings) = expr {} to be written as if expr is Pat(bindings) {} (RFC 3573). if let chains are a natural extension of the already existing if let syntax, and I'd argue orthogonal towards is syntax.
  • One could have similar chaining inside let ... else statements. There is no proposed RFC for this however, nor is it implemented on nightly.
  • Match guards have the if keyword as well, but on stable Rust, they don't support let. The functionality is available via an unstable feature (if_let_guard tracking issue). Stabilization of let chains affects this feature in so far as match guards containing let chains now only need the if_let_guard feature gate be present instead of also the let_chains feature (NOTE: this PR doesn't implement this simplification, it's left for future work).

Open questions / blockers

@est31 est31 added the F-let_chains `#![feature(let_chains)]` label Nov 10, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Nov 10, 2024
@jieyouxu jieyouxu added T-lang Relevant to the language team, which will review and decide on the PR/issue. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC A-edition-2024 Area: The 2024 edition and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Nov 10, 2024
@traviscross
Copy link
Contributor

traviscross commented Nov 10, 2024

Process-wise, we normally do the FCP on the stabilization PR, and I think that'd make the most sense here. Let us know if you need us on lang to weigh in on anything specifically, e.g. any of the open questions, to make such a stabilization PR possible.

cc @rust-lang/lang for visibility.

And cc @rust-lang/style given the open item on the style guide.

@rustbot labels +I-style-nominated

Putting on the edition hat, since this would be an edition item in some sense, we should talk about this in our next edition call.

@rustbot labels +I-edition-nominated

@rustbot rustbot added I-style-nominated Nominated for discussion during a style team meeting. I-edition-nominated Nominated for discussion during an edition team meeting. labels Nov 10, 2024
@rustbot
Copy link
Collaborator

rustbot commented Nov 10, 2024

r? @fee1-dead

rustbot has assigned @fee1-dead.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 10, 2024
@est31 est31 removed the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Nov 10, 2024
@est31
Copy link
Member Author

est31 commented Nov 10, 2024

Process-wise, we normally do the FCP on the stabilization PR, and I think that'd make the most sense here.

@traviscross understood, I've converted it to a pull request using the gh cli. Thanks for cc'ing the relevant teams.

@traviscross
Copy link
Contributor

Beautiful, thanks. Let's nominate this for lang discussion too then.

@rustbot labels +I-lang-nominated

@rustbot rustbot added the I-lang-nominated Nominated for discussion during a lang team meeting. label Nov 10, 2024
@rust-log-analyzer

This comment has been minimized.

@fee1-dead fee1-dead self-assigned this Nov 11, 2024
@compiler-errors compiler-errors added the S-waiting-on-fcp Status: PR is in FCP and is awaiting for FCP to complete. label Nov 11, 2024
@compiler-errors compiler-errors removed their assignment Nov 11, 2024
@fee1-dead fee1-dead added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 12, 2024
@traviscross
Copy link
Contributor

@rustbot labels -I-edition-nominated

We talked this one through in the edition call. Since there's no breakage or migration here, we're happy for this one to land in Rust 2024 either ahead of or after the release of Rust 2024 itself as long as there is an edition guide chapter for it.

@rustbot rustbot removed the I-edition-nominated Nominated for discussion during an edition team meeting. label Nov 13, 2024
@est31 est31 force-pushed the stabilize_let_chains branch from aa24aa5 to d57626e Compare November 13, 2024 09:55
Copy link
Member

@fee1-dead fee1-dead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I also understanding this correctly that this PR will also stablize the following on all editions?

fn main() {
    let x: Option<Option<i32>> = None;
    while let Some(x) = x && let Some(_) = x {}
}

Has there been any discussion to stabilize that? My understanding is that only 2024 was discussed and FCP'd, but maybe I missed something. If only 2024 was discussed then we probably can't stabilize any form of let chains with edition < 2024.

Comment on lines 4039 to 4045
/// Whether let chains are allowed on all editions, or it's edition dependent.
/// In case of edition dependence, specify the currently present edition.
pub enum LetChainsPolicy {
AlwaysAllowed,
Edition(Edition),
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// Whether let chains are allowed on all editions, or it's edition dependent.
/// In case of edition dependence, specify the currently present edition.
pub enum LetChainsPolicy {
AlwaysAllowed,
Edition(Edition),
}
/// Whether let chains are allowed on all editions, or it's edition dependent.
/// In case of edition dependence, specify the currently present edition.
pub enum LetChainsContext {
AlwaysAllowed,
EditionDependent { current_edition: Edition },
}

When you write it as LetChainsPolicy it feels like the edition that you are passing in is some sort of condition that must be passed, as compared to the current edition. I believe this change would help clarify things better.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changed

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 19, 2025
@est31 est31 force-pushed the stabilize_let_chains branch from b4bd8b5 to d69f6e8 Compare April 19, 2025 22:36
@est31
Copy link
Member Author

est31 commented Apr 19, 2025

Has there been any discussion to stabilize that? My understanding is that only 2024 was discussed and FCP'd, but maybe I missed something.

No you are right, only 2024 has been discussed. The reason why if let chains are stabilized on 2024 doesn't apply to while though, so such a case can be made. But it still needs to be discussed. I don't think this stabilization PR should be held up more than necessary, so I've removed that part. It can happen in a later PR.

@est31 est31 force-pushed the stabilize_let_chains branch from d69f6e8 to 5258cb7 Compare April 20, 2025 21:15
@@ -3425,14 +3429,9 @@ impl<'a> Parser<'a> {
let if_span = self.prev_token.span;
let mut cond = self.parse_match_guard_condition()?;

CondChecker::new(self).visit_expr(&mut cond);
CondChecker::new(self, LetChainsPolicy::AlwaysAllowed).visit_expr(&mut cond);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be nice to leave a note here that this is allowed since it is already gated behind if_let_guard, so it is still unstable and not being stabilized. But that can be done as a followup.

@fee1-dead
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 22, 2025

📌 Commit 5258cb7 has been approved by fee1-dead

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 22, 2025
@bors
Copy link
Collaborator

bors commented Apr 22, 2025

⌛ Testing commit 5258cb7 with merge 8bf5a8d...

@bors
Copy link
Collaborator

bors commented Apr 22, 2025

☀️ Test successful - checks-actions
Approved by: fee1-dead
Pushing 8bf5a8d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Apr 22, 2025
@bors bors merged commit 8bf5a8d into rust-lang:master Apr 22, 2025
7 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Apr 22, 2025
Copy link

What is this? This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.

Comparing 9bfa31f (parent) -> 8bf5a8d (this PR)

Test differences

Show 16 test diffs

Stage 1

  • [ui] tests/ui/drop/issue-100276.rs: pass -> [missing] (J0)
  • [ui] tests/ui/drop/issue-100276.rs#edition2021: [missing] -> pass (J0)
  • [ui] tests/ui/drop/issue-100276.rs#edition2024: [missing] -> pass (J0)
  • [ui] tests/ui/rfcs/rfc-2497-if-let-chains/edition-gate-macro-error.rs#edition2021: [missing] -> pass (J0)
  • [ui] tests/ui/rfcs/rfc-2497-if-let-chains/edition-gate-macro-error.rs#edition2024: [missing] -> pass (J0)
  • [ui] tests/ui/rfcs/rfc-2497-if-let-chains/edition-gate-macro.rs#edition2021: [missing] -> pass (J0)
  • [ui] tests/ui/rfcs/rfc-2497-if-let-chains/edition-gate-macro.rs#edition2024: [missing] -> pass (J0)

Stage 2

  • [ui] tests/ui/drop/issue-100276.rs: pass -> [missing] (J1)
  • [ui] tests/ui/drop/issue-100276.rs#edition2021: [missing] -> pass (J1)
  • [ui] tests/ui/drop/issue-100276.rs#edition2024: [missing] -> pass (J1)
  • [ui] tests/ui/rfcs/rfc-2497-if-let-chains/edition-gate-macro-error.rs#edition2021: [missing] -> pass (J1)
  • [ui] tests/ui/rfcs/rfc-2497-if-let-chains/edition-gate-macro-error.rs#edition2024: [missing] -> pass (J1)
  • [ui] tests/ui/rfcs/rfc-2497-if-let-chains/edition-gate-macro.rs#edition2021: [missing] -> pass (J1)
  • [ui] tests/ui/rfcs/rfc-2497-if-let-chains/edition-gate-macro.rs#edition2024: [missing] -> pass (J1)

Additionally, 2 doctest diffs were found. These are ignored, as they are noisy.

Job group index

Test dashboard

Run

cargo run --manifest-path src/ci/citool/Cargo.toml -- \
    test-dashboard 8bf5a8d12feea10dfada53fb2d119283b0e0107c --output-dir test-dashboard

And then open test-dashboard/index.html in your browser to see an overview of all executed tests.

Job duration changes

  1. x86_64-apple-2: 4820.0s -> 5488.8s (13.9%)
  2. aarch64-apple: 3816.6s -> 4320.5s (13.2%)
  3. dist-apple-various: 8806.0s -> 7932.1s (-9.9%)
  4. dist-aarch64-apple: 4610.6s -> 5043.8s (9.4%)
  5. aarch64-gnu-debug: 3958.6s -> 4239.4s (7.1%)
  6. dist-x86_64-apple: 8865.3s -> 9279.8s (4.7%)
  7. i686-gnu-1: 8354.2s -> 8724.8s (4.4%)
  8. i686-gnu-nopt-1: 8268.3s -> 8596.6s (4.0%)
  9. i686-msvc-2: 7236.4s -> 6959.1s (-3.8%)
  10. dist-loongarch64-musl: 5311.2s -> 5480.4s (3.2%)
How to interpret the job duration changes?

Job durations can vary a lot, based on the actual runner instance
that executed the job, system noise, invalidated caches, etc. The table above is provided
mostly for t-infra members, for simpler debugging of potential CI slow-downs.

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (8bf5a8d): comparison URL.

Overall result: ✅ improvements - no action needed

@rustbot label: -perf-regression

Instruction count

This is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-0.3% [-0.4%, -0.2%] 3
All ❌✅ (primary) - - 0

Max RSS (memory usage)

Results (primary -1.1%, secondary 2.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
2.6% [2.0%, 3.2%] 2
Regressions ❌
(secondary)
2.7% [2.7%, 2.7%] 1
Improvements ✅
(primary)
-3.5% [-6.5%, -1.4%] 3
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -1.1% [-6.5%, 3.2%] 5

Cycles

Results (primary -0.7%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-0.7% [-0.7%, -0.7%] 1
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) -0.7% [-0.7%, -0.7%] 1

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 775.125s -> 776.303s (0.15%)
Artifact size: 365.14 MiB -> 365.05 MiB (-0.02%)

@traviscross traviscross added the relnotes Marks issues that should be documented in the release notes of the next release. label Apr 22, 2025
Hofer-Julian added a commit to Hofer-Julian/pixi that referenced this pull request Apr 23, 2025
This will enable us to use let chains in the future among other things: rust-lang/rust#132833
@est31
Copy link
Member Author

est31 commented Apr 23, 2025

The PR finally merged. If there is no reverts or delays, this means that the feature will ship to stable in the 1.88.0 release, arriving to stable users on June 26, 2025.

💖 Thanks for everyone who has participated in making let chains happen 💖

It is a feature that's simple in syntax but it still touched a lot of teams, and has hidden complexities (drop behavior).

Zalathar added a commit to Zalathar/rust that referenced this pull request Apr 24, 2025
… r=lcnr

Make #![feature(let_chains)] bootstrap conditional in compiler/

Let chains have been stabilized recently in rust-lang#132833, so we can remove the gating from our uses in the compiler (as the compiler uses edition 2024).
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Apr 25, 2025
… r=lcnr

Make #![feature(let_chains)] bootstrap conditional in compiler/

Let chains have been stabilized recently in rust-lang#132833, so we can remove the gating from our uses in the compiler (as the compiler uses edition 2024).
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Apr 25, 2025
Rollup merge of rust-lang#140202 - est31:let_chains_feature_compiler, r=lcnr

Make #![feature(let_chains)] bootstrap conditional in compiler/

Let chains have been stabilized recently in rust-lang#132833, so we can remove the gating from our uses in the compiler (as the compiler uses edition 2024).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2024 Area: The 2024 edition disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. F-let_chains `#![feature(let_chains)]` finished-final-comment-period The final comment period is finished for this PR / Issue. I-lang-radar Items that are on lang's radar and will need eventual work or consideration. merged-by-bors This PR was explicitly merged by bors. relnotes Marks issues that should be documented in the release notes of the next release. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-lang Relevant to the language team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tracking issue for eRFC 2497, "if- and while-let-chains, take 2"